[Settings, ListCommand] Add sort types, settings infrastructure, and CLI arguments for output ordering#6177
Conversation
…gs infrastructure ## Summary Introduce configurable output sorting for `winget list` as part of issue the sorting feature request. This is part 1 of a 3-PR stack that lays the type system and settings foundation without changing any runtime behavior. ## Changes - **SortField and SortDirection enums**: Add `SortField` (Relevance, Name, Id, Version, Source, Available) and `SortDirection` (Ascending, Descending) to the settings type system. Relevance is internal-only and not exposed as a valid settings value. - **Settings entries**: Register `OutputSortOrder` and `OutputSortDirection` with typed validation that maps JSON string values to enums using case-insensitive comparison. Default sort order is `[Name]` ascending. Invalid field names reject the entire setting and fall back to default. - **JSON schema**: Extend `settings.schema.0.2.json` with an `output` section containing `sortOrder` (string array) and `sortDirection` (string enum with ascending default). - **Unit tests**: Add 16 test sections covering default values, single and multi-field configurations, case insensitivity, empty array (disables sorting), invalid fields, mixed valid/invalid, and wrong JSON types. ## Impact - No behavioral change ╬ô├ç├╢ settings are defined but not yet consumed by any command or workflow. Existing `winget list` output remains unchanged. - Users who edit `settings.json` to add `output.sortOrder` or `output.sortDirection` will have their values validated and stored, but the values will not take effect until PR3 wires the sort logic. ## How Validated - Unit tests added (16 test sections across SettingOutputSortOrder and SettingOutputSortDirection test cases) - Compilation verified via msbuild for AppInstallerCommonCore and AppInstallerCLITests (compile-only)
…dators Follow established codebase pattern (NetworkDownloader, LoggingLevel) of declaring static constexpr std::string_view variables for string comparisons instead of inline string literals.
314ebb6 to
edeb3bb
Compare
This comment has been minimized.
This comment has been minimized.
Add test case verifying behavior when duplicate field names appear in sortOrder settings (e.g., ["name", "id", "Name"]). Duplicates are accepted and preserved — case-insensitive matching maps both "name" and "Name" to SortField::Name. Addresses reviewer feedback from PR microsoft#6176.
edeb3bb to
abebe97
Compare
JohnMcPMS
left a comment
There was a problem hiding this comment.
This includes the previous change, and I don't think there is enough in them to justify a split PR (and they have some shared portions, like which values are valid).
… relevance, lowercase-once - Change OutputSortOrder default from [Name] to empty vector (sentinel for context-aware defaults: relevance when query, Name when no query) - Make Relevance a publicly visible sort field in settings and schema - Refactor OutputSortOrder validator to ToLower once then compare with == - Update tests for new default, add relevance field test section
…guments ## Summary Register CLI arguments for output sorting on the list command as part of the sorting feature request. This is part 2 of a 3-PR stack that adds the argument definitions without wiring them to any sort logic. ## Changes - **Argument types**: Add Sort, SortAscending, and SortDescending to the Args::Type enum under the List Command section. - **Argument registration**: Map Sort to `--sort` (Standard arg), SortAscending to `--ascending`/`--asc` (Flag), and SortDescending to `--descending`/`--desc` (Flag) in ArgumentCommon::ForType. Ascending and descending share a SortDirection exclusive set to enforce mutual exclusivity. - **ListCommand integration**: Add all three arguments to ListCommand::GetArguments() with descriptive resource strings. - **Resource strings**: Add SortArgumentDescription, SortAscendingArgumentDescription, and SortDescendingArgumentDescription with localization comments to winget.resw and corresponding IDs to Resources.h. ## Impact - No behavioral change ╬ô├ç├╢ arguments are accepted and parsed but not consumed by any workflow. Existing `winget list` output is unchanged. - The existing EnsureAllArgumentsDefined test will automatically verify all three new arg types have valid ForType mappings. - `--ascending` and `--descending` cannot be used together due to the SortDirection exclusive set constraint. ## How Validated - Compilation verified via msbuild for AppInstallerCLICore (full build) and AppInstallerCLITests (compile-only)
abebe97 to
f15a3e1
Compare
|
Thanks John — understood. I split them as stacked PRs for independent scoping, but agree the change here is small. The latest PR1 fixes (shared |
|
I don't see a shared function here or in the other PR. Can we just close one of the PRs? |
Add a shared string-to-SortField converter so CLI argument parsing and workflow logic can resolve sort field names consistently. - Declare ConvertToSortField in UserSettings.h (public API) - Implement with case-insensitive matching via Utility::ToLower - Add Catch2 tests: lowercase, case-insensitive, invalid, round-trip Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Sorry for the confusion — |
## Problem - Schema description for `sortOrder` used "disable sorting" which didn't accurately describe the behavior of an empty array - The `--sort` argument description in winget.resw was unnecessarily verbose and included implementation details in the comment - The `OutputSortOrder` validator duplicated the same field-name-to-enum parsing logic that `ConvertToSortField` already provides ## Solution - Update schema description to "An empty array results in default sorting" - Shorten sort argument description to "Sort results by a property (can be repeated)" and simplify the comment - Refactor `OutputSortOrder` validator to call `ConvertToSortField()` instead of maintaining its own inline if/else chain with local constexpr strings ## Impact - Eliminates ~30 lines of duplicated string-to-enum parsing logic in the validator - Future sort field additions only need to update `ConvertToSortField` — the validator follows automatically Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Implements the sort logic for `winget list` output, completing the feature requested in #4238. Builds on PR #6177 (settings parsing + CLI handling). ### Changes **Sort orchestration** (`WorkflowBase.cpp`): - `SortInstalledPackagesTableLines` — resolves sort fields from CLI `--sort` > settings `output.sortOrder` > query-aware default, determines direction, sorts via `std::stable_sort`, applies permutation back. - When the free-text query (`-q`) is used and no sort preference is configured, relevance ordering is preserved. If `output.sortOrder` is configured, it is respected even with queries. - `FAIL_FAST_MSG` for unrecognized sort field values — ensures invalid enum values crash loudly in debug builds. **Sort helper** (`PackageTableSortHelper.h/.cpp`): - `SortablePackageEntry` — holds pre-computed case-folded sortable fields, computed only for fields needed via `ComputeSortFieldMask` bitmask optimization. - `CompareByField` — case-insensitive ordinal comparison using pre-computed `FoldCase` values. - `SortBy` — template sort executor used by the workflow; standalone `SortEntries` wrapper available for unit tests. - `SortField` enum uses flag-bit values (`uint32_t`) with `DEFINE_ENUM_FLAG_OPERATORS` for efficient bitmask field tracking. **CLI integration** (`ListCommand.cpp`): - `--sort` (repeatable, limit 6), `--ascending`/`--asc`, `--descending`/`--desc` arguments. **Argument validation** (`Command.cpp`): - Validates `--sort` values in `ValidateArguments`, following the `--scope`/`--authentication-mode` pattern. **Documentation** (`list.md`): - `--sort`, `--ascending`, `--descending` in options table. New "Sorting output" section. ###⚠️ Behavior change: default sort order Previously, `winget list` output preserved the source's natural ordering (source-determined relevance ranking). **This PR changes the default to sort alphabetically by name (ascending)** when no `--sort` argument, no `-q` query, and no `output.sortOrder` setting is configured. | Scenario | Behavior | |----------|----------| | No query, no settings, no `--sort` | Sorts by name (ascending) — **new default** | | `-q` query with no sort preference | Preserves source relevance ordering (unchanged) | | `--sort relevance` or `"sortOrder": ["relevance"]` | Preserves source ordering explicitly | | `"sortOrder": []` (empty array) | Same as no setting — applies default name sort | | `output.sortOrder` configured + `-q` query | Settings override relevance (explicit user preference wins) | **To restore previous (source-determined) ordering:** - CLI: `winget list --sort relevance` - Settings: `{ "output": { "sortOrder": ["relevance"] } }` > **Note:** An empty `sortOrder` array is equivalent to the setting not being present — it does not disable sorting. Use `"relevance"` explicitly to preserve source ordering. ### Design decisions | Decision | Rationale | |----------|-----------| | **Default sort by name** | When no settings and no query, sort alphabetically by name — most natural default for browsing installed packages. | | **Relevance preservation with queries** | Free-text query (`-q`) without configured sort preserves source relevance ranking. Settings override this if configured — explicit user preference takes priority. | | **`stable_sort`** | Preserves relative order for multi-field sorting (e.g., `--sort source --sort name` groups by source, alphabetical within). | | **Flag-bit `SortField` enum** | Enables `ComputeSortFieldMask` to skip unused field extraction — measurable win for large package lists. | | **Pre-computed `FoldCase`** | Expensive case-folding done once per entry at construction, not on every comparison call. | | **`FAIL_FAST_MSG` on invalid sort** | Catches enum drift at development time rather than silent misbehavior. | ### How validated **Unit tests** — 20 test cases: - 18 sort tests in `PackageTableSortHelper.cpp`: single/multi-field sorting, both directions, edge cases, case-insensitive comparison, relevance no-op - 1 drift detection test in `Command.cpp`: reads `ListCommand::GetArguments()` limit and validates against known `ConvertToSortField` strings — breaks if enum changes without updating limit - 1 enforcement test in `Command.cpp`: verifies `SetCountLimit(6)` rejects 7 `--sort` values with `TooManyArgError` **Manual testing** — 21 scenarios on `wingetdev` (Debug x64): defaults, single/multi-field sorts, direction flags, query + sort interaction, settings precedence, edge cases, invalid input. All passing. Fixes #4238 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Add settings infrastructure and CLI arguments for configurable output sorting in
winget list. This lays the full type system, settings plumbing, and argumentregistration without changing any runtime behavior — sort logic is wired in a
follow-up PR.
Part of #4238
Changes
Settings types and schema
SortField(Relevance, Name, Id, Version,Source, Available) and
SortDirection(Ascending, Descending) to the settings typesystem. Relevance preserves the current natural ordering and is publicly configurable
(users can set
["relevance"]to explicitly opt out of sorting).OutputSortOrderandOutputSortDirectionwith typedvalidation using lowercase-once comparison (
Utility::ToLower+==). Default sortorder is empty (
{}) — an empty vector acts as a sentinel meaning "use context-awaredefaults" (the follow-up PR's sort logic decides: relevance for queries, Name for
no-query).
settings.schema.0.2.jsonwith anoutputsection containingsortOrder(string array with relevance, name, id, version, source, available) andsortDirection(string enum with ascending default).follow the established
static constexpr std::string_viewpattern used by existingvalidators.
CLI arguments
Sort,SortAscending, andSortDescendingarg types toExecutionArgs.h.--sort(multi-use, max 7 values),--ascending/--asc,and
--descending/--descinArgument.cppwith resource string descriptions.--ascendingand--descendingare mutually exclusive viaExclusiveOf.ListCommand::GetArguments().winget.reswand resource IDs inResources.h.Impact
any workflow. Existing
winget listoutput remains unchanged.winget list --sort nameis accepted without error but output remains unsorted untilthe sort logic PR lands.
--ascendingand--descendingare mutually exclusive — passing both produces an error.How Validated
SettingOutputSortOrderandSettingOutputSortDirectioncovering default values (empty fallback), single/multi-field, case insensitivity, empty
array, relevance field, duplicate fields, invalid fields, mixed valid/invalid, and wrong
JSON types (68 assertions total).
wingetdev list --sort nameaccepted,wingetdev list --helpshows new arguments.
Follow-up
comprehensive sort tests will be in a follow-up PR.